Skip to content

AI SDK Backend#2336

Merged
sawka merged 14 commits intomainfrom
sawka/ai-sdk
Sep 12, 2025
Merged

AI SDK Backend#2336
sawka merged 14 commits intomainfrom
sawka/ai-sdk

Conversation

@sawka
Copy link
Member

@sawka sawka commented Sep 12, 2025

Working on AI SDK compatible backends for OpenAI and Anthropic. Thinking + ToolUse etc.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

Adds SSE-based streaming infrastructure, provider adapters, CLI test tooling, schemas, docs, and web routing changes to support useChat-compatible AI streaming. New channel-driven SSE handler (pkg/waveai/ssehandlerch.go) and AI message helpers are introduced. Provider integrations for Anthropic and OpenAI streaming (multiple files under pkg/waveai) map provider events to the SSE protocol. A top-level useChat handler and routing (pkg/waveai/usechat.go, pkg/web/web.go) resolve AI config and route requests. A CLI test harness and JSON test schema are added (cmd/testai/). Documentation files describing streaming protocols and backend design are added (aiprompts/.md). go.mod updated for OpenAI client and indirect deps.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "AI SDK Backend" is short, clear, and directly related to the primary change set which implements AI-SDK–compatible backends for OpenAI and Anthropic along with SSE streaming and handler plumbing, so it accurately conveys the main intent to a reviewer scanning history.
Description Check ✅ Passed The description states the work is on AI SDK compatible backends for OpenAI and Anthropic and references Thinking and ToolUse, which is clearly related to the changes in the patch and satisfies the lenient description check.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/ai-sdk

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
pkg/waveai/googlebackend.go (3)

48-51: Never return a nil channel on errors; emit a single error on a real channel to avoid deadlocks.

Current early-return paths return nil, which can block forever or panic at call sites expecting a readable channel.

Apply this diff to return a non-nil channel carrying the error (and consider removing the unreachable nil-model check in this SDK):

 func (GoogleBackend) StreamCompletion(ctx context.Context, request wshrpc.WaveAIStreamRequest) chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType] {
@@
-  client, err := genai.NewClient(ctx, clientOptions...)
-  if err != nil {
-    log.Printf("failed to create client: %v", err)
-    return nil
-  }
+  client, err := genai.NewClient(ctx, clientOptions...)
+  if err != nil {
+    rtn := make(chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType])
+    go func() {
+      defer close(rtn)
+      rtn <- makeAIError(fmt.Errorf("failed to create Google client: %v", err))
+    }()
+    return rtn
+  }
@@
-  model := client.GenerativeModel(request.Opts.Model)
-  if model == nil {
-    log.Println("model not found")
-    client.Close()
-    return nil
-  }
+  model := client.GenerativeModel(request.Opts.Model)
+  // genai.GenerativeModel typically does not return nil; treat unknown model as runtime error later.

Also applies to: 54-58


60-66: Validate that prompt is non-empty before accessing the last element.

Accessing the last message without checking length can panic when request.Prompt is empty.

Apply this diff to fail fast with a clear error:

-  cs := model.StartChat()
+  if len(request.Prompt) == 0 {
+    client.Close()
+    rtn := make(chan wshrpc.RespOrErrorUnion[wshrpc.WaveAIPacketType])
+    go func() {
+      defer close(rtn)
+      rtn <- makeAIError(fmt.Errorf("prompt must contain at least one message"))
+    }()
+    return rtn
+  }
+  cs := model.StartChat()

91-107: Guard helper functions against empty slices to prevent panics.

Both helpers assume at least one element; add length checks to be defensive.

Apply this diff:

 func extractHistory(history []wshrpc.WaveAIPromptMessageType) []*genai.Content {
   var rtn []*genai.Content
-  for _, h := range history[:len(history)-1] {
+  if len(history) <= 1 {
+    return nil
+  }
+  for _, h := range history[:len(history)-1] {
     if h.Role == "user" || h.Role == "model" {
       rtn = append(rtn, &genai.Content{
         Role:  h.Role,
         Parts: []genai.Part{genai.Text(h.Content)},
       })
     }
   }
   return rtn
 }
 
 func extractPrompt(prompt []wshrpc.WaveAIPromptMessageType) genai.Part {
-  p := prompt[len(prompt)-1]
-  return genai.Text(p.Content)
+  if len(prompt) == 0 {
+    return genai.Text("")
+  }
+  p := prompt[len(prompt)-1]
+  return genai.Text(p.Content)
 }
🧹 Nitpick comments (24)
pkg/waveai/googlebackend.go (2)

38-45: Add sane HTTP transport timeouts when configuring a custom proxy client.

The custom client currently lacks dial/TLS timeouts, which can hang connections on network issues. Keep stream-friendly semantics by avoiding a client-wide Timeout, but set transport-level timeouts.

Apply this diff and add imports for net and time:

+  // Optional: tighten transport timeouts while preserving streaming behavior.
+  // Requires: import "net" and "time".
   transport := &http.Transport{
-    Proxy: http.ProxyURL(proxyURL),
+    Proxy: http.ProxyURL(proxyURL),
+    TLSHandshakeTimeout: 10 * time.Second,
+    ExpectContinueTimeout: 1 * time.Second,
+    IdleConnTimeout: 90 * time.Second,
+    DialContext: (&net.Dialer{
+      Timeout:   10 * time.Second,
+      KeepAlive: 30 * time.Second,
+    }).DialContext,
   }
   httpClient := &http.Client{
     Transport: transport,
   }

109-117: Defensive nil checks when flattening candidates.

Some SDK responses can include nil candidates or nil Content; skip safely.

Apply this diff:

 func convertCandidatesToText(candidates []*genai.Candidate) string {
   var rtn string
   for _, c := range candidates {
-    for _, p := range c.Content.Parts {
+    if c == nil || c.Content == nil {
+      continue
+    }
+    for _, p := range c.Content.Parts {
       rtn += fmt.Sprintf("%v", p)
     }
   }
   return rtn
 }
go.mod (1)

26-26: Consider using the sashabaranov/go-openai library consistently

The module adds both github.com/openai/openai-go/v2 (line 26) and already has github.com/sashabaranov/go-openai (line 27). Having two different OpenAI client libraries in the same project could lead to confusion and maintenance overhead.

Consider standardizing on one OpenAI client library throughout the codebase. The sashabaranov library appears to already be in use and supports streaming, so you might not need the additional openai-go/v2 dependency unless it provides specific features you require.

aiprompts/aisdk-streaming.md (1)

82-82: Format URLs properly in examples

The bare URLs in the examples should be wrapped in backticks or formatted as proper Markdown links for better readability.

-data: {"type":"source-url","sourceId":"https://example.com","url":"https://example.com"}
+data: {"type":"source-url","sourceId":"https://example.com","url":"https://example.com"}

The same applies to lines 90 and 98.

Also applies to: 90-90, 98-98

aiprompts/usechat-backend-design.md (1)

10-16: Add language specifiers to fenced code blocks

Several code blocks are missing language specifiers, which affects syntax highlighting and readability.

For example, line 10 should be:

-```
+```text

Or use appropriate language identifiers like http for endpoint definitions and text for plain text blocks.

Also applies to: 19-25, 32-34, 37-39, 145-150, 153-163, 173-176, 439-442, 445-449

pkg/waveai/ssehandlerch.go (2)

129-130: Consider making keepalive interval configurable

The keepalive ticker is hardcoded to 1 second. Some proxies or clients might need different intervals.

Consider adding a configurable keepalive interval:

+type SSEHandlerOptions struct {
+    KeepaliveInterval time.Duration
+}
+
-func MakeSSEHandlerCh(w http.ResponseWriter, ctx context.Context) *SSEHandlerCh {
+func MakeSSEHandlerCh(w http.ResponseWriter, ctx context.Context, opts *SSEHandlerOptions) *SSEHandlerCh {
+    if opts == nil {
+        opts = &SSEHandlerOptions{
+            KeepaliveInterval: SSEKeepaliveInterval,
+        }
+    }

235-242: Consider non-blocking writes to prevent deadlock

The WriteData method could block if the write channel is full and the context isn't done. This uses a default case that returns an error, but in high-throughput scenarios, you might want to consider a more graceful degradation.

Consider implementing a timeout or exponential backoff for writes:

 select {
 case h.writeCh <- SSEMessage{Type: SSEMsgData, Data: data}:
     return nil
 case <-h.ctx.Done():
     return h.ctx.Err()
-default:
-    return fmt.Errorf("write channel is full")
+case <-time.After(100 * time.Millisecond):
+    return fmt.Errorf("write timeout: channel is full")
 }
pkg/waveai/usechat-openai-responses.go (2)

19-80: Consider adding validation for empty input items list.

The function correctly handles message conversion and empty content filtering, but it doesn't validate whether the resulting inputItems list is empty after filtering. This could lead to an API error if all messages are filtered out.

Add validation after filtering:

 	// Convert messages to input items, filtering out empty content
 	var inputItems []responses.ResponseInputItemUnionParam
 	for _, msg := range messages {
 		content := msg.GetContent()
 		// Skip messages with empty content as OpenAI requires non-empty content
 		if strings.TrimSpace(content) == "" {
 			continue
 		}
 
 		// Convert role to EasyInputMessageRole
 		var role responses.EasyInputMessageRole
 		switch msg.Role {
 		case "user":
 			role = responses.EasyInputMessageRoleUser
 		case "assistant":
 			role = responses.EasyInputMessageRoleAssistant
 		case "system":
 			role = responses.EasyInputMessageRoleSystem
 		default:
 			role = responses.EasyInputMessageRoleUser
 		}
 
 		inputItems = append(inputItems, responses.ResponseInputItemParamOfMessage(content, role))
 	}
+
+	// Ensure we have at least one input item
+	if len(inputItems) == 0 {
+		inputItems = append(inputItems, responses.ResponseInputItemParamOfMessage("Hello", responses.EasyInputMessageRoleUser))
+	}

176-194: Redundant cleanup code in response.completed handler.

The cleanup code for ending reasoning and text streams is duplicated in the response.completed handler (lines 176-185) when it's already handled by the response.output_item.done event. This could lead to duplicate end events being sent.

Consider removing the redundant cleanup since response.output_item.done should have already handled it:

 		case "response.completed":
 			responseDone := event.AsResponseCompleted()
 			if !finished {
 				usage := &OpenAIUsageResponse{}
 				responseUsage := responseDone.Response.Usage
 				usage.PromptTokens = int(responseUsage.InputTokens)
 				usage.CompletionTokens = int(responseUsage.OutputTokens)
 				usage.TotalTokens = usage.PromptTokens + usage.CompletionTokens
 
-				// End reasoning if it was started but not ended
-				if reasoningStarted && !reasoningEnded {
-					sseHandler.AiMsgReasoningEnd(reasoningId)
-					reasoningEnded = true
-				}
-				// End text if it was started but not ended
-				if textStarted && !textEnded {
-					sseHandler.AiMsgTextEnd(textId)
-					textEnded = true
-				}
-
 				finishReason := "stop"
 				if responseDone.Response.Status == "completed" {
 					finishReason = "stop"
 				}
 
 				sseHandler.AiMsgFinish(finishReason, usage)
 				finished = true
 			}
 			return
aiprompts/anthropic-streaming.md (1)

373-476: Fix inconsistent budget_tokens value in thinking example.

The shell example uses "budget_tokens": 16000 (line 390) while the Python example uses "budget_tokens": 16000 (line 411). These are consistent, but the response shows a much smaller thinking output than would be expected with a 16000 token budget.

Consider updating the example to show a more realistic budget_tokens value or expanding the response to better demonstrate the thinking process with the allocated budget.

cmd/testai/main-testai.go (1)

60-89: Add error handling for missing schema properties.

The getToolDefinitions function assumes the schema structure exists but doesn't validate if the "config" key is present in the unmarshaled schema.

Add validation for the config schema:

 	configSchema := schemas["config"]
+	if configSchema == nil {
+		fmt.Printf("Warning: 'config' schema not found in testschema.json\n")
+		configSchema = map[string]interface{}{
+			"type": "object",
+		}
+	}
pkg/web/web.go (1)

476-494: Consider extracting CORS configuration to constants.

The CORS configuration in dev mode is hardcoded inline. Consider extracting these values to constants for better maintainability.

Extract CORS configuration to constants:

+const (
+	DevCORSMethods = "GET, POST, PUT, DELETE, OPTIONS"
+	DevCORSHeaders = "Content-Type, X-Session-Id, X-AuthKey, Authorization, X-Requested-With, Accept, x-vercel-ai-ui-message-stream"
+	DevCORSExposeHeaders = "X-ZoneFileInfo, Content-Length, Content-Type, x-vercel-ai-ui-message-stream"
+)
+
 	if wavebase.IsDevMode() {
 		originalHandler := handler
 		handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			origin := r.Header.Get("Origin")
 			if origin != "" {
 				w.Header().Set("Access-Control-Allow-Origin", origin)
 			}
-			w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
-			w.Header().Set("Access-Control-Allow-Headers", "Content-Type, X-Session-Id, X-AuthKey, Authorization, X-Requested-With, Accept, x-vercel-ai-ui-message-stream")
-			w.Header().Set("Access-Control-Expose-Headers", "X-ZoneFileInfo, Content-Length, Content-Type, x-vercel-ai-ui-message-stream")
+			w.Header().Set("Access-Control-Allow-Methods", DevCORSMethods)
+			w.Header().Set("Access-Control-Allow-Headers", DevCORSHeaders)
+			w.Header().Set("Access-Control-Expose-Headers", DevCORSExposeHeaders)
 			w.Header().Set("Access-Control-Allow-Credentials", "true")
pkg/waveai/usechat-openai-completions.go (1)

41-90: Consider adding validation for empty chatMessages list.

Similar to the Responses API implementation, this function should validate that chatMessages is not empty after filtering.

Add validation after filtering:

 		}
 	}
+
+	// Ensure we have at least one message
+	if len(chatMessages) == 0 {
+		chatMessages = append(chatMessages, openai.UserMessage("Hello"))
+	}
 
 	// Create request using Chat Completions API
pkg/waveai/usechat.go (6)

91-95: Simplify error handling for JSON unmarshaling.

The current error handling pattern silently ignores unmarshaling errors. Consider logging or handling the error more explicitly rather than setting to nil.

-		presetAiSettings = &wconfig.AiSettingsType{}
-		if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil {
-			// Successfully unmarshaled preset
-		} else {
-			presetAiSettings = nil
-		}
+		presetAiSettings = &wconfig.AiSettingsType{}
+		if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err != nil {
+			// Log the error for debugging purposes
+			log.Printf("Failed to unmarshal preset %s: %v", presetKey, err)
+			presetAiSettings = nil
+		}

102-106: Apply consistent error handling pattern for block metadata unmarshaling.

Similar to the preset unmarshaling above, this should follow the same error handling pattern.

-	blockAiSettings = &wconfig.AiSettingsType{}
-	if err := json.Unmarshal(mustMarshal(block.Meta), blockAiSettings); err != nil {
-		blockAiSettings = nil
-	}
+	blockAiSettings = &wconfig.AiSettingsType{}
+	if err := json.Unmarshal(mustMarshal(block.Meta), blockAiSettings); err != nil {
+		// Log the error for debugging purposes
+		log.Printf("Failed to unmarshal block metadata for block %s: %v", blockId, err)
+		blockAiSettings = nil
+	}

125-127: Consider making the default model configurable or using a constant.

The hardcoded model "gpt-4.1" might not be available or appropriate for all users. Consider making this configurable or at least defining it as a constant.

Define a constant at the package level:

const DefaultAIModel = "gpt-4o-mini"

Then use it here:

-	if aiOpts.Model == "" {
-		aiOpts.Model = "gpt-4.1"
-	}
+	if aiOpts.Model == "" {
+		aiOpts.Model = DefaultAIModel
+	}

167-169: Potential null pointer dereference in generateID.

The function doesn't check the error from rand.Read. While extremely unlikely to fail, it's good practice to handle it.

 func generateID() string {
 	bytes := make([]byte, 16)
-	rand.Read(bytes)
+	if _, err := rand.Read(bytes); err != nil {
+		// Fallback to a timestamp-based ID if random generation fails
+		return fmt.Sprintf("%d", time.Now().UnixNano())
+	}
 	return hex.EncodeToString(bytes)
 }

Add the import at the top:

import (
	...
	"time"
	...
)

210-213: Improve API type validation logic.

The current condition is confusing with the double negative. Consider restructuring for clarity and maintainability.

-	// Support OpenAI and Anthropic
-	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
-		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
-		return
-	}
+	// Validate API type - support OpenAI and Anthropic
+	switch aiOpts.APIType {
+	case APIType_OpenAI, APIType_Anthropic, "":
+		// Valid API types (empty defaults to OpenAI)
+	default:
+		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+		return
+	}

231-234: Unify streaming error handling across providers.

StreamAnthropicResponses returns (*StopReason, error) (pkg/waveai/usechat-anthropic.go) and its error is checked/logged at pkg/waveai/usechat.go:229–236, while OpenAI streamers (pkg/waveai/usechat-openai-completions.go / pkg/waveai/usechat-openai-responses.go) return no error. Choose one consistent approach: either make the OpenAI stream functions return (*StopReason, error) and propagate/handle errors in pkg/waveai/usechat.go, or have pkg/waveai/usechat.go treat Anthropic the same as OpenAI (ignore/handle the returned error consistently); ensure SSE error events are emitted uniformly.

pkg/waveai/usechat-anthropic.go (5)

1-10: Clean up documentation comments.

The comments contain reference markers (:contentReference[oaicite:X]{index=X}) that appear to be from an internal documentation system. These should be removed or replaced with actual references for clarity.

-// Package anthropicadapter streams Anthropic Messages API events and adapts them
-// to our AI‑SDK style SSE parts. Mapping is based on the AI‑SDK data stream
-// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0}
-// and Anthropic's Messages + Streaming event schemas (message_start,
-// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2}
-//
-// NOTE: The public signature in api.txt references wshrpc.WaveAIOptsType;
-// for this self-contained package we define WaveAIOptsType locally with the
-// same shape. Adapt the import/alias as needed in your codebase. :contentReference[oaicite:3]{index=3}
+// Package waveai contains the Anthropic streaming adapter that converts Anthropic Messages API events 
+// to AI-SDK style SSE parts. Mapping is based on the AI-SDK data stream
+// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step)
+// and Anthropic's Messages + Streaming event schemas (message_start,
+// content_block_start/delta/stop, message_delta, message_stop, error).

308-309: Consider making buffer sizes configurable.

The hardcoded buffer sizes (64KB initial, 8MB max) might not be suitable for all use cases. Consider making these configurable or at least defining them as constants.

+const (
+	// SSE scanner buffer sizes
+	sseInitialBufferSize = 64 * 1024     // 64KB
+	sseMaxBufferSize     = 8 * 1024 * 1024 // 8MB
+)

 	// Stream decoding state
 	scanner := bufio.NewScanner(resp.Body)
-	scanner.Buffer(make([]byte, 64*1024), 8*1024*1024) // allow large lines
+	scanner.Buffer(make([]byte, sseInitialBufferSize), sseMaxBufferSize)

342-355: Simplify nested condition logic.

The nested if-else structure for handling event results is complex. Consider simplifying for better readability.

-			if curEvent != "" {
-				if stop, ret, rerr := handleAnthropicEvent(curEvent, dataBuf.String(), sse, blockMap, &toolCalls, &msgID, &model, finalStop); rerr != nil {
-					// Anthropic sent error event or malformed JSON.
-					return stop, rerr
-				} else if ret != nil {
-					// message_stop triggered return
-					return ret, nil
-				} else {
-					// maybe updated final stop reason (from message_delta)
-					if stop != nil && stop.RawReason != "" {
-						finalStop = stop.RawReason
-					}
-				}
-			}
+			if curEvent != "" {
+				stop, ret, rerr := handleAnthropicEvent(curEvent, dataBuf.String(), sse, blockMap, &toolCalls, &msgID, &model, finalStop)
+				
+				if rerr != nil {
+					// Anthropic sent error event or malformed JSON.
+					return stop, rerr
+				}
+				
+				if ret != nil {
+					// message_stop triggered return
+					return ret, nil
+				}
+				
+				// maybe updated final stop reason (from message_delta)
+				if stop != nil && stop.RawReason != "" {
+					finalStop = stop.RawReason
+				}
+			}

641-645: Simplify empty content handling.

The condition checking and reassigning empty content is redundant.

 		} else {
 			// Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25}
-			if m.Content == "" {
-				m.Content = ""
-			}
 			aim.Content = json.RawMessage(fmt.Sprintf("%q", m.Content))
 		}

652-656: Handle potential error from rand.Read.

Similar to the generateID function in usechat.go, this should handle the potential error from rand.Read.

 func genID(prefix string) string {
 	var b [8]byte
-	_, _ = rand.Read(b[:])
+	if _, err := rand.Read(b[:]); err != nil {
+		// Fallback to timestamp-based ID if random generation fails
+		return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano())
+	}
 	return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:]))
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2783eeb and b9e3a84.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (14)
  • aiprompts/aisdk-streaming.md (1 hunks)
  • aiprompts/anthropic-streaming.md (1 hunks)
  • aiprompts/usechat-backend-design.md (1 hunks)
  • cmd/testai/main-testai.go (1 hunks)
  • cmd/testai/testschema.json (1 hunks)
  • go.mod (2 hunks)
  • pkg/waveai/googlebackend.go (1 hunks)
  • pkg/waveai/ssehandlerch.go (1 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat-openai-completions.go (1 hunks)
  • pkg/waveai/usechat-openai-responses.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
  • pkg/waveai/waveai.go (2 hunks)
  • pkg/web/web.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
pkg/waveai/ssehandlerch.go (9)
  • SSEHandlerCh (63-75)
  • AiMsgStart (38-38)
  • AiMsgReasoningStart (42-42)
  • AiMsgReasoningDelta (43-43)
  • AiMsgTextStart (39-39)
  • AiMsgTextDelta (40-40)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgFinish (51-51)
pkg/waveai/usechat-openai-completions.go (1)
  • OpenAIUsageResponse (35-39)
pkg/wavebase/wavebase.go (1)
  • IsDevMode (102-104)
pkg/waveai/usechat-openai-completions.go (3)
pkg/waveai/ssehandlerch.go (6)
  • SSEHandlerCh (63-75)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgTextDelta (40-40)
  • AiMsgTextEnd (41-41)
  • AiMsgFinish (51-51)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
pkg/waveai/waveai.go (2)
pkg/waveai/anthropicbackend.go (2)
  • AnthropicBackend (21-21)
  • AnthropicBackend (113-316)
pkg/waveai/perplexitybackend.go (2)
  • PerplexityBackend (21-21)
  • PerplexityBackend (53-193)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
  • SSEHandlerCh (63-75)
  • AiMsgError (52-52)
  • AiMsgFinish (51-51)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgReasoningStart (42-42)
  • AiMsgToolInputStart (45-45)
  • AiMsgTextDelta (40-40)
  • AiMsgReasoningDelta (43-43)
  • AiMsgToolInputDelta (46-46)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgToolInputAvailable (47-47)
  • AiMsgFinishStep (50-50)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat-anthropic.go (2)
  • ToolDefinition (62-66)
  • StreamAnthropicResponses (197-382)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (2)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (155-164)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat.go (9)
pkg/wconfig/settingsconfig.go (2)
  • AiSettingsType (35-52)
  • MergeAiSettings (165-229)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/wstore/wstore_dbops.go (1)
  • DBMustGet (138-149)
pkg/wconfig/filewatcher.go (1)
  • GetWatcher (33-57)
pkg/waveai/waveai.go (2)
  • APIType_OpenAI (21-21)
  • APIType_Anthropic (18-18)
pkg/waveai/ssehandlerch.go (2)
  • SSEHandlerCh (63-75)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-openai-completions.go (1)
  • StreamOpenAIChatCompletions (41-162)
pkg/waveai/usechat-openai-responses.go (1)
  • StreamOpenAIResponsesAPI (82-225)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (197-382)
pkg/web/web.go (4)
pkg/waveai/usechat.go (1)
  • HandleAIChat (172-239)
pkg/docsite/docsite.go (1)
  • GetDocsiteHandler (17-31)
pkg/schema/schema.go (1)
  • GetSchemaHandler (17-31)
pkg/wavebase/wavebase.go (1)
  • IsDevMode (102-104)
🪛 markdownlint-cli2 (0.17.2)
aiprompts/anthropic-streaming.md

23-23: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

aiprompts/aisdk-streaming.md

82-82: Bare URL used

(MD034, no-bare-urls)


90-90: Bare URL used

(MD034, no-bare-urls)


98-98: Bare URL used

(MD034, no-bare-urls)

aiprompts/usechat-backend-design.md

10-10: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


19-19: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


32-32: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


37-37: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


145-145: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


153-153: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


173-173: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


439-439: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


445-445: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (19)
pkg/waveai/googlebackend.go (1)

1-3: License header addition looks good.

SPDX and copyright are correctly placed above the package declaration.

go.mod (1)

85-88: tidwall deps are transitive — keep as indirect

No direct imports found in the repo; go mod graph / go list show github.com/openai/openai-go/v2@v2.1.1 pulls in github.com/tidwall/{gjson,match,pretty,sjson}, so leaving them as indirect in go.mod (lines 85–88) is correct.

pkg/waveai/waveai.go (2)

18-19: LGTM! Good API naming standardization

The renaming from ApiType_ to APIType_ follows Go naming conventions where acronyms should be consistently capitalized.


84-89: LGTM! Consistent application of naming convention

The constant references have been properly updated to use the new naming convention throughout the function.

cmd/testai/testschema.json (1)

1-104: Well-structured test schema

The JSON schema file is well-organized with proper validation constraints, including:

  • Appropriate min/max values for numeric fields
  • Pattern validation for repository and workflow file names
  • Clear separation between configuration and data schemas
  • Proper use of JSON Schema features ($ref, definitions)
aiprompts/aisdk-streaming.md (1)

1-186: Comprehensive SSE protocol documentation

The documentation effectively describes the Data Stream Protocol with:

  • Clear explanation of each message type and its purpose
  • Proper SSE format examples
  • Support for advanced features (reasoning, tool use, multi-modal)
  • Compatibility notes for frontend hooks

This aligns well with the implementation in the SSE handler code.

aiprompts/usechat-backend-design.md (1)

1-463: Excellent architectural design document

This is a well-thought-out design that:

  • Clearly explains the migration from RPC to HTTP/SSE
  • Provides concrete implementation examples
  • Addresses configuration resolution comprehensively
  • Includes a sensible phased migration strategy
  • Demonstrates significant complexity reduction (90% frontend code reduction)

The approach of proxying provider SSE streams directly rather than converting back and forth is particularly elegant.

pkg/waveai/ssehandlerch.go (2)

314-329: LGTM! Well-implemented graceful shutdown

The Close method properly handles concurrent access with appropriate locking, closes the write channel to trigger the [DONE] message, and waits for the writer goroutine to complete. This ensures clean shutdown without data loss.


1-440: Well-structured SSE handler implementation

Overall, this is a solid implementation of a channel-based SSE handler with:

  • Good separation of concerns between writing and message handling
  • Proper concurrency control with mutexes
  • Automatic keepalive handling
  • Clean shutdown mechanism
  • Comprehensive AI message helper methods

The implementation aligns well with the documented protocol and provides a robust foundation for the AI streaming functionality.

pkg/waveai/usechat-openai-responses.go (2)

1-17: LGTM! License header and imports are properly structured.

The copyright notice and imports are appropriate for the functionality being implemented.


205-209: LGTM! Proper error handling for stream errors.

The error handling correctly captures stream errors and sends them through the SSE handler before returning.

aiprompts/anthropic-streaming.md (1)

1-40: LGTM! Comprehensive documentation for Anthropic streaming.

The documentation clearly explains the streaming setup with good SDK examples for both Python and TypeScript.

cmd/testai/main-testai.go (2)

20-22: LGTM! Proper use of go:embed for test schema.

The embedded test schema is a clean way to include test data without external file dependencies.


178-209: LGTM! Well-structured command-line interface.

The main function provides clear usage instructions and appropriate error handling for missing arguments and environment variables.

pkg/web/web.go (2)

406-412: LGTM! Proper CORS preflight handling.

The addition of OPTIONS request handling without auth validation is correct for CORS preflight requests.


452-474: Good separation of concerns with router architecture.

The separation of routes into waveRouter and vdomRouter with appropriate timeout handling is well-structured. The /api/aichat endpoint correctly bypasses timeout handling for streaming support.

pkg/waveai/usechat-openai-completions.go (3)

1-15: LGTM! License header and imports are properly structured.

The copyright notice and imports are appropriate for the Chat Completions API implementation.


108-144: LGTM! Clean streaming implementation.

The streaming logic properly handles text deltas and finish reasons with appropriate state tracking.


153-162: LGTM! Proper cleanup handling.

The cleanup logic correctly ensures that any started text stream is properly ended before sending the finish message.

Comment on lines +582 to +583
event: content_block_stop
data: {"type":"content_block_stop","index":1 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix formatting issue in JSON response.

There's a missing space before the closing brace on line 583.

-data: {"type":"content_block_stop","index":1 }
+data: {"type":"content_block_stop","index":1}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
event: content_block_stop
data: {"type":"content_block_stop","index":1 }
event: content_block_stop
data: {"type":"content_block_stop","index":1}
🤖 Prompt for AI Agents
In aiprompts/anthropic-streaming.md around lines 582 to 583, the JSON line
`data: {"type":"content_block_stop","index":1 }` has a missing space before the
closing brace; update the JSON formatting to include a space before the `}` (or
otherwise make spacing consistent) so the line reads with proper spacing, e.g.,
ensure there's a space before the closing brace after the index value.

Comment on lines +233 to +273
// Example: Direct OpenAI streaming (minimal conversion)
func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) {
client := openai.NewClient(opts.APIToken)

stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{
Model: opts.Model,
Messages: convertToOpenAIMessages(messages),
Stream: true,
})
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
defer stream.Close()

for {
response, err := stream.Recv()
if errors.Is(err, io.EOF) {
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}

// Direct transformation: OpenAI format → useChat format
for _, choice := range response.Choices {
if choice.Delta.Content != "" {
fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content)
}
if choice.FinishReason != "" {
fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason)
}
}

w.(http.Flusher).Flush()
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential resource leak in streamOpenAIToUseChat

The function creates a stream but only defers stream.Close() after checking for an error. If CreateChatCompletionStream succeeds but an error occurs before the defer statement, the stream won't be closed.

 func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) {
     client := openai.NewClient(opts.APIToken)
     
     stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{
         Model:    opts.Model,
         Messages: convertToOpenAIMessages(messages),
         Stream:   true,
     })
+    if stream != nil {
+        defer stream.Close()
+    }
     if err != nil {
         fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
         fmt.Fprintf(w, "data: [DONE]\n\n")
         return
     }
-    defer stream.Close()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Example: Direct OpenAI streaming (minimal conversion)
func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) {
client := openai.NewClient(opts.APIToken)
stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{
Model: opts.Model,
Messages: convertToOpenAIMessages(messages),
Stream: true,
})
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
defer stream.Close()
for {
response, err := stream.Recv()
if errors.Is(err, io.EOF) {
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
// Direct transformation: OpenAI format → useChat format
for _, choice := range response.Choices {
if choice.Delta.Content != "" {
fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content)
}
if choice.FinishReason != "" {
fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason)
}
}
w.(http.Flusher).Flush()
}
}
// Example: Direct OpenAI streaming (minimal conversion)
func streamOpenAIToUseChat(w http.ResponseWriter, ctx context.Context, opts *WaveAIOptsType, messages []Message) {
client := openai.NewClient(opts.APIToken)
stream, err := client.CreateChatCompletionStream(ctx, openai.ChatCompletionRequest{
Model: opts.Model,
Messages: convertToOpenAIMessages(messages),
Stream: true,
})
if stream != nil {
defer stream.Close()
}
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
for {
response, err := stream.Recv()
if errors.Is(err, io.EOF) {
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
if err != nil {
fmt.Fprintf(w, "data: {\"type\":\"error\",\"error\":%q}\n\n", err.Error())
fmt.Fprintf(w, "data: [DONE]\n\n")
return
}
// Direct transformation: OpenAI format → useChat format
for _, choice := range response.Choices {
if choice.Delta.Content != "" {
fmt.Fprintf(w, "data: {\"type\":\"text\",\"text\":%q}\n\n", choice.Delta.Content)
}
if choice.FinishReason != "" {
fmt.Fprintf(w, "data: {\"type\":\"finish\",\"finish_reason\":%q}\n\n", choice.FinishReason)
}
}
w.(http.Flusher).Flush()
}
}
🤖 Prompt for AI Agents
aiprompts/usechat-backend-design.md around lines 233 to 273: the stream returned
by CreateChatCompletionStream must be closed on every exit path; ensure you call
defer stream.Close() immediately after confirming err==nil (no statements
between creation and defer) so the stream is always closed, and for any other
early-return error paths that may occur before the defer place an explicit
stream.Close() (or restructure into a small helper/closure that defers close) so
no code path leaks the stream.

Comment on lines +366 to +371
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
return h.WriteJsonData(resp)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Unused parameters in AiMsgFinish

The finishReason and usage parameters are accepted but not included in the response map.

 func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
     resp := map[string]interface{}{
         "type": AiMsgFinish,
     }
+    if finishReason != "" {
+        resp["finishReason"] = finishReason
+    }
+    if usage != nil {
+        resp["usage"] = usage
+    }
     return h.WriteJsonData(resp)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
return h.WriteJsonData(resp)
}
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
if finishReason != "" {
resp["finishReason"] = finishReason
}
if usage != nil {
resp["usage"] = usage
}
return h.WriteJsonData(resp)
}
🤖 Prompt for AI Agents
In pkg/waveai/ssehandlerch.go around lines 366 to 371, AiMsgFinish currently
ignores its finishReason and usage parameters; update the response map to
include them (for example add "finish_reason": finishReason and "usage": usage),
ensure usage is JSON-serializable (handle nil by omitting or setting to nil
explicitly), then call h.WriteJsonData(resp) as before so the finish reason and
usage are sent in the SSE payload.

Comment on lines +217 to +225
endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
apiVersion := opts.APIVersion
if apiVersion == "" {
apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12}
}
maxTokens := opts.MaxTokens
if maxTokens <= 0 {
maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for base URL format.

The endpoint construction should validate the base URL format to prevent potential issues with malformed URLs.

 	baseURL := opts.BaseURL
 	if baseURL == "" {
 		baseURL = "https://api.anthropic.com"
 	}
-	endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
+	// Validate and normalize base URL
+	parsedURL, err := url.Parse(baseURL)
+	if err != nil {
+		return nil, fmt.Errorf("invalid base URL: %w", err)
+	}
+	endpoint := strings.TrimRight(parsedURL.String(), "/") + "/v1/messages"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
apiVersion := opts.APIVersion
if apiVersion == "" {
apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12}
}
maxTokens := opts.MaxTokens
if maxTokens <= 0 {
maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13}
}
baseURL := opts.BaseURL
if baseURL == "" {
baseURL = "https://api.anthropic.com"
}
// Validate and normalize base URL
parsedURL, err := url.Parse(baseURL)
if err != nil {
return nil, fmt.Errorf("invalid base URL: %w", err)
}
endpoint := strings.TrimRight(parsedURL.String(), "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
apiVersion := opts.APIVersion
if apiVersion == "" {
apiVersion = "2023-06-01" // default from examples :contentReference[oaicite:12]{index=12}
}
maxTokens := opts.MaxTokens
if maxTokens <= 0 {
maxTokens = 1024 // safe default per docs/examples :contentReference[oaicite:13]{index=13}
}
🤖 Prompt for AI Agents
In pkg/waveai/usechat-anthropic.go around lines 217 to 225, the code currently
concatenates baseURL to build endpoint without validating baseURL; add
validation by parsing baseURL with net/url (url.Parse), ensure it has a valid
scheme (http or https) and host, normalize by trimming any trailing slash or
using url.ResolveReference to join the path, and if parsing/validation fails
return or propagate an error instead of constructing the endpoint; update the
surrounding function to handle and return this error if not already handled.

Comment on lines +82 to +102
func StreamOpenAIResponsesAPI(sseHandler *SSEHandlerCh, ctx context.Context, opts *wshrpc.WaveAIOptsType, messages []UseChatMessage) {
client, req := createOpenAIRequest(opts, messages)

// Create stream using Responses API
stream := client.Responses.NewStreaming(ctx, req)
defer stream.Close()

// Generate IDs for the streaming protocol
messageId := generateID()
textId := generateID()
reasoningId := generateID()

// Send message start
sseHandler.AiMsgStart(messageId)

// Track whether we've started text/reasoning streaming and finished
textStarted := false
textEnded := false
reasoningStarted := false
reasoningEnded := false
finished := false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Clean up debug print statements before production.

There are multiple debug print statements that should be removed or gated behind a debug flag before production deployment.

Remove or properly gate the debug statements:

-	fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+	if wavebase.IsDevMode() {
+		fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+	}

Apply similar changes to lines 109, 124, and 130.

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 30

🧹 Nitpick comments (9)
aiprompts/usechat-backend-design.md (2)

10-16: Specify language for fenced code blocks.

The architecture comparison diagrams should specify a language for better rendering.

 ### Current Architecture
-```
+```text
 Frontend (React) → Custom RPC → Go Backend → AI Providers
 - 10+ Jotai atoms for state management
 - Custom WaveAIStreamRequest/WaveAIPacketType
 - Complex configuration merging in frontend
 - Custom streaming protocol over WebSocket

Target Architecture

- +text
Frontend (useChat) → HTTP/SSE → Go Backend → AI Providers

  • Single useChat() hook manages all state
  • Standard HTTP POST + SSE streaming
  • Backend-driven configuration resolution
  • Standard AI SDK streaming format

Also applies to: 19-25


32-40: Add language specifiers to remaining code blocks.

Several code blocks are missing language specifiers which helps with syntax highlighting and readability.

 **Chat Streaming Endpoint:**
-```
+```text
 POST /api/ai/chat/{blockId}?preset={presetKey}

Conversation Persistence Endpoints:
- +text
POST /api/ai/conversations/{blockId} # Save conversation
GET /api/ai/conversations/{blockId} # Load conversation


**Headers:**
-```
+```http
Content-Type: text/event-stream
Cache-Control: no-cache
Connection: keep-alive
Access-Control-Allow-Origin: *

useChat Expected Format:
- +text
data: {"type":"text","text":"Hello"}

data: {"type":"text","text":" world"}

data: {"type":"text","text":"!"}

data: {"type":"finish","finish_reason":"stop","usage":{"prompt_tokens":10,"completion_tokens":3,"total_tokens":13}}

data: [DONE]


**Error Format:**
-```
+```text
data: {"type":"error","error":"API key invalid"}

data: [DONE]

URL-based Configuration

- +text
POST /api/ai/chat/block-123?preset=claude-coding
POST /api/ai/chat/block-456?preset=gpt4-creative


### Header-based Overrides
-```
+```http
POST /api/ai/chat/block-123
X-AI-Model: gpt-4-turbo
X-AI-Temperature: 0.8


Also applies to: 145-150, 153-177, 439-461

</blockquote></details>
<details>
<summary>pkg/waveai/usechat-openai-responses.go (1)</summary><blockquote>

`187-191`: **Simplify redundant finish reason logic**

The finish reason assignment is redundant - it always sets "stop" regardless of the condition.

```diff
-				finishReason := "stop"
-				if responseDone.Response.Status == "completed" {
-					finishReason = "stop"
-				}
+				finishReason := "stop"
+				// TODO: Map different response statuses to appropriate finish reasons
cmd/testai/testschema.json (1)

17-17: Non-standard JSON Schema property

The units property is not part of the JSON Schema specification. Consider using the description field to document units or use a custom annotation with an x- prefix.

-				"units": "s"
+				"x-units": "s"

Or include the unit in the description:

-				"description": "Polling interval for GitHub API requests",
+				"description": "Polling interval for GitHub API requests (in seconds)",
cmd/testai/main-testai.go (1)

167-168: Remove trailing whitespace

There's trailing whitespace on line 168.

 		tools = getToolDefinitions()
 	}
-	
+
 	stopReason, err := waveai.StreamAnthropicResponses(ctx, sseHandler, opts, messages, tools)
pkg/waveai/usechat.go (1)

91-95: Improve error handling for preset unmarshaling

The error from JSON unmarshaling is silently ignored. Consider logging the error for debugging purposes.

 			presetAiSettings = &wconfig.AiSettingsType{}
 			if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil {
 				// Successfully unmarshaled preset
 			} else {
+				log.Printf("Warning: Failed to unmarshal preset %s: %v", presetKey, err)
 				presetAiSettings = nil
 			}
pkg/waveai/usechat-anthropic.go (1)

308-309: Consider potential scanner buffer overflow for very large SSE events.

Setting a large maximum buffer size (8MB) is good, but the code doesn't handle the case where a line exceeds even this limit.

Consider logging a warning or handling this edge case explicitly if scanner.Err() returns bufio.ErrTooLong.

pkg/waveai/ssehandlerch.go (2)

217-222: Potential race condition with error channel.

The error channel has a buffer of 1, but the comment says "Send error to error channel if there's space" with a non-blocking select. If multiple errors occur concurrently, only the first will be captured in errCh. Consider if this is the intended behavior or if you need a larger buffer or different error handling strategy.


241-242: Inconsistent error messages for full channel conditions.

The error message "write channel is full" could be more helpful by indicating that this is likely a backpressure issue.

 	default:
-		return fmt.Errorf("write channel is full")
+		return fmt.Errorf("SSE write channel is full (backpressure)")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c47b82 and f944df9.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (14)
  • aiprompts/aisdk-streaming.md (1 hunks)
  • aiprompts/anthropic-streaming.md (1 hunks)
  • aiprompts/usechat-backend-design.md (1 hunks)
  • cmd/testai/main-testai.go (1 hunks)
  • cmd/testai/testschema.json (1 hunks)
  • go.mod (2 hunks)
  • pkg/waveai/googlebackend.go (1 hunks)
  • pkg/waveai/ssehandlerch.go (1 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat-openai-completions.go (1 hunks)
  • pkg/waveai/usechat-openai-responses.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
  • pkg/waveai/waveai.go (2 hunks)
  • pkg/web/web.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
pkg/waveai/ssehandlerch.go (9)
  • SSEHandlerCh (63-75)
  • AiMsgStart (38-38)
  • AiMsgReasoningStart (42-42)
  • AiMsgReasoningDelta (43-43)
  • AiMsgTextStart (39-39)
  • AiMsgTextDelta (40-40)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgFinish (51-51)
pkg/waveai/usechat-openai-completions.go (1)
  • OpenAIUsageResponse (35-39)
pkg/wavebase/wavebase.go (1)
  • IsDevMode (102-104)
pkg/waveai/usechat-openai-completions.go (3)
pkg/waveai/ssehandlerch.go (6)
  • SSEHandlerCh (63-75)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgTextDelta (40-40)
  • AiMsgTextEnd (41-41)
  • AiMsgFinish (51-51)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
pkg/waveai/waveai.go (2)
pkg/waveai/anthropicbackend.go (2)
  • AnthropicBackend (21-21)
  • AnthropicBackend (113-316)
pkg/waveai/perplexitybackend.go (2)
  • PerplexityBackend (21-21)
  • PerplexityBackend (53-193)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
  • SSEHandlerCh (63-75)
  • AiMsgError (52-52)
  • AiMsgFinish (51-51)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgReasoningStart (42-42)
  • AiMsgToolInputStart (45-45)
  • AiMsgTextDelta (40-40)
  • AiMsgReasoningDelta (43-43)
  • AiMsgToolInputDelta (46-46)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgToolInputAvailable (47-47)
  • AiMsgFinishStep (50-50)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat-anthropic.go (2)
  • ToolDefinition (62-66)
  • StreamAnthropicResponses (197-382)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (2)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (155-164)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat.go (9)
pkg/wconfig/settingsconfig.go (2)
  • AiSettingsType (35-52)
  • MergeAiSettings (165-229)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/wstore/wstore_dbops.go (1)
  • DBMustGet (138-149)
pkg/wconfig/filewatcher.go (1)
  • GetWatcher (33-57)
pkg/waveai/waveai.go (2)
  • APIType_OpenAI (21-21)
  • APIType_Anthropic (18-18)
pkg/waveai/ssehandlerch.go (2)
  • SSEHandlerCh (63-75)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-openai-completions.go (1)
  • StreamOpenAIChatCompletions (41-162)
pkg/waveai/usechat-openai-responses.go (1)
  • StreamOpenAIResponsesAPI (82-225)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (197-382)
pkg/web/web.go (2)
pkg/waveai/usechat.go (1)
  • HandleAIChat (172-239)
pkg/wavebase/wavebase.go (1)
  • IsDevMode (102-104)
🪛 markdownlint-cli2 (0.17.2)
aiprompts/usechat-backend-design.md

10-10: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


19-19: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


32-32: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


37-37: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


145-145: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


153-153: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


173-173: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


439-439: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


445-445: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (15)
pkg/waveai/googlebackend.go (1)

1-3: LGTM! Copyright header added correctly.

The addition of the copyright and SPDX license header is appropriate and follows standard practices.

go.mod (2)

85-88: Indirect tidwall dependencies look appropriate for JSON handling.

The tidwall libraries (gjson, sjson, match, pretty) are commonly used for JSON manipulation and are reasonable indirect dependencies for the streaming infrastructure.


26-26: Action: Confirm and consolidate OpenAI SDK usage

Both SDKs are actually imported:

  • pkg/waveai/openaibackend.go — imports github.com/sashabaranov/go-openai (alias openaiapi)
  • pkg/waveai/usechat-openai-completions.go — imports github.com/openai/openai-go/v2 (and option)
  • pkg/waveai/usechat-openai-responses.go — imports github.com/openai/openai-go/v2 (and responses/shared)

Actions:

  • Confirm both SDKs are intentional; if not, remove the unused SDK from go.mod and update imports.
  • Prefer consolidating to a single SDK for consistency; if both must remain, add comments documenting the reason and verify no conflicting configs (API keys, base URLs, duplicated behavior).
pkg/waveai/waveai.go (2)

18-19: Good standardization of API constant naming.

The renaming from ApiType_ to APIType_ follows Go naming conventions where acronyms should be consistently capitalized.


84-89: Backend type assignment is consistent with the renamed constants.

The updates to use APIType_Anthropic and APIType_Perplexity are correctly applied, maintaining the same functional behavior while improving naming consistency.

aiprompts/aisdk-streaming.md (1)

1-186: Comprehensive SSE streaming protocol documentation.

This documentation provides excellent coverage of the SSE-based streaming protocol with clear examples for each message type. The protocol design aligns well with the implementation in the codebase and supports multiple AI provider integrations.

aiprompts/anthropic-streaming.md (1)

1-632: Excellent Anthropic streaming documentation with error recovery.

The documentation thoroughly covers Anthropic's streaming API including tool use, extended thinking, and importantly, error recovery strategies. The examples are comprehensive and will be valuable for implementation and debugging.

aiprompts/usechat-backend-design.md (1)

183-310: Well-structured implementation examples.

The Go implementation examples provide excellent guidance for the HTTP handler and streaming logic. The direct proxy approach for OpenAI/Anthropic and the conversion only for Wave Cloud is an efficient design choice.

pkg/web/web.go (5)

31-31: Good integration of the AI chat functionality.

The addition of the waveai package import is appropriate for supporting the new AI chat endpoint.


408-412: CORS preflight handling is correctly implemented.

The addition of CORS preflight handling in WebFnWrap properly responds to OPTIONS requests without auth validation, which is essential for browser-based clients.


453-466: Good separation of routes with different timeout requirements.

Creating separate routers for wave and vdom endpoints with appropriate timeout handlers is a clean architectural improvement. This prevents timeout issues for long-running operations while maintaining protection for standard endpoints.


469-469: AI chat endpoint correctly configured without timeout.

The /api/aichat endpoint is appropriately configured without a timeout handler, which is necessary for SSE streaming that can run for extended periods.


477-494: Approve dev-only CORS behavior — IsDevMode checks WAVETERM_DEV env var

IsDevMode returns true when Dev_VarCache (populated from os.Getenv(WaveDevVarName) and then unset) is non-empty — see pkg/wavebase/wavebase.go:52, 96–104; dynamic Access-Control-Allow-Origin is therefore gated to explicit dev mode.

pkg/waveai/usechat.go (1)

146-153: Define isReasoningModel function

This file references isReasoningModel function in shouldUseChatCompletionsAPI but doesn't define it. This is the missing function that's causing compilation issues in other files.

The isReasoningModel function needs to be defined. Add it to this file or create a shared utility file:

+func isReasoningModel(model string) bool {
+	m := strings.ToLower(model)
+	// Models with reasoning capabilities (o3, o4, etc.)
+	return strings.HasPrefix(m, "o3") || 
+		strings.HasPrefix(m, "o4") ||
+		strings.Contains(m, "reasoning")
+}
+
 func shouldUseChatCompletionsAPI(model string) bool {

Likely an incorrect or invalid review comment.

pkg/waveai/usechat-anthropic.go (1)

234-238: Add proper cancellation of timeout context.

The timeout context should be cancelled on all return paths to ensure resources are properly cleaned up, not just deferred.

 	// Context with timeout if provided.
 	if opts.TimeoutMs > 0 {
 		var cancel context.CancelFunc
 		ctx, cancel = context.WithTimeout(ctx, time.Duration(opts.TimeoutMs)*time.Millisecond)
 		defer cancel()
 	}

Likely an incorrect or invalid review comment.

Comment on lines +366 to +371
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
return h.WriteJsonData(resp)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing finishReason and usage parameters in AiMsgFinish.

The function accepts finishReason and usage parameters but doesn't include them in the response map. This could lead to missing information in the SSE stream.

 func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
 	resp := map[string]interface{}{
 		"type": AiMsgFinish,
 	}
+	if finishReason != "" {
+		resp["finishReason"] = finishReason
+	}
+	if usage != nil {
+		resp["usage"] = usage
+	}
 	return h.WriteJsonData(resp)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
return h.WriteJsonData(resp)
}
func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
resp := map[string]interface{}{
"type": AiMsgFinish,
}
if finishReason != "" {
resp["finishReason"] = finishReason
}
if usage != nil {
resp["usage"] = usage
}
return h.WriteJsonData(resp)
}
🤖 Prompt for AI Agents
In pkg/waveai/ssehandlerch.go around lines 366 to 371, the AiMsgFinish function
currently ignores its finishReason and usage parameters; update the response map
to include these values (e.g., add keys "finish_reason": finishReason and
"usage": usage or use the existing key naming convention in the file), so the
SSE payload contains both fields before calling WriteJsonData; ensure nil usage
is handled (can be included as-is) and no other call sites need changing.

Comment on lines +108 to +109
fmt.Printf("DEBUG: Received event type: %s\n", event.Type)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove or conditionally compile debug output

Debug print statements should be removed or wrapped in a debug mode check to avoid cluttering production logs.

-		fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+		if wavebase.IsDevMode() {
+			fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
if wavebase.IsDevMode() {
fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
}
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around lines 108-109, the
unconditional fmt.Printf debug statement should be removed or guarded; either
delete the print or replace it with a proper logger debug call (e.g.,
logger.Debugf("Received event type: %s", event.Type)) or wrap it in a debug-mode
check/read-from-ENV flag (if os.Getenv("DEBUG") == "1") so it only executes in
debug builds/environments; ensure no stray fmt.Printf remains in production
code.

Comment on lines +124 to +125
fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
if reasoningDelta.Delta != "" && !reasoningEnded {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove or conditionally compile debug output

Debug print statements should be removed or controlled by debug mode flag.

-			fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+			if wavebase.IsDevMode() {
+				fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
if reasoningDelta.Delta != "" && !reasoningEnded {
if wavebase.IsDevMode() {
fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
}
if reasoningDelta.Delta != "" && !reasoningEnded {
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around lines 124 to 125, the
fmt.Printf debug output should not be printed unconditionally; either remove it
or guard it behind a debug flag or logger. Replace the direct fmt.Printf call
with a conditional check against an existing debug/logging flag (or add one if
missing) and use the package logger or a debug-level log method so the message
is emitted only when debug mode is enabled, or delete the statement entirely if
no debug telemetry is required.

}

case "response.reasoning_summary_text.done":
fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Remove or conditionally compile debug output

Debug print statements should be removed or controlled by debug mode flag.

-			fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+			if wavebase.IsDevMode() {
+				fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
if wavebase.IsDevMode() {
fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
}
🤖 Prompt for AI Agents
In pkg/waveai/usechat-openai-responses.go around line 130, there is a raw
fmt.Printf used for debugging; remove this unconditional stdout print or guard
it behind a debug mode check (use an existing debug/config flag or add a
package-level debug boolean) and replace with a proper debug-level logger call
(e.g., logger.Debugf) or only execute the fmt.Printf when the debug flag is
true; ensure imports and callers reflect the change so no stray debug output
remains in production builds.

Comment on lines +167 to +169
bytes := make([]byte, 16)
rand.Read(bytes)
return hex.EncodeToString(bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potential panic from uninitialized random source

The rand.Read function may not be properly initialized. Use crypto/rand which is already imported but ensure error handling.

 func generateID() string {
 	bytes := make([]byte, 16)
-	rand.Read(bytes)
+	if _, err := rand.Read(bytes); err != nil {
+		// Fallback to timestamp-based ID or handle error
+		return fmt.Sprintf("%d", time.Now().UnixNano())
+	}
 	return hex.EncodeToString(bytes)
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 167–169, replace the call to
math/rand.Read with crypto/rand.Read and handle its returned error;
specifically, call crypto/rand.Read(bytes), check err, and propagate the error
to the caller (preferably change the function signature to return (string,
error)) or, if changing the signature is impractical, log the error and return a
safe zero value instead of ignoring it.

Comment on lines +210 to +213
if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Inconsistent API type validation

The validation allows empty APIType but then checks for specific values. This could lead to unexpected behavior.

-	// Support OpenAI and Anthropic
-	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
-		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+	// Validate API type - default was already set to OpenAI if empty
+	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
+		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
 		return
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
return
}
// Validate API type - default was already set to OpenAI if empty
if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
return
}
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 210 to 213, the APIType check currently
allows an empty string which is inconsistent; update validation to explicitly
reject empty APIType (or alternatively set a clear default) and return a Bad
Request when aiOpts.APIType == ""; then simplify the conditional to only allow
APIType == APIType_OpenAI or APIType_Anthropic and return an error listing the
allowed values when validation fails.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (7)
pkg/waveai/usechat.go (3)

166-170: Handle crypto/rand.Read error in generateID.

Avoid ignoring entropy errors; add a safe fallback.

 func generateID() string {
 	bytes := make([]byte, 16)
-	rand.Read(bytes)
+	if _, err := rand.Read(bytes); err != nil {
+		// fallback: time-based id (low collision risk for our usage)
+		return fmt.Sprintf("%x", time.Now().UnixNano())
+	}
 	return hex.EncodeToString(bytes)
 }

Also add:

-import (
+import (
+	"time"

209-213: Tighten API type validation (empty should not pass).

-// Support OpenAI and Anthropic
-if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
-	http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+// Support OpenAI and Anthropic only
+if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
+	http.Error(w, fmt.Sprintf("Unsupported API type: %q (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
 	return
 }

55-61: Preserve ErrNotFound end-to-end; return 404 (not 500).

You now wrap with %w (good), but HandleAIChat treats all resolveAIConfig errors as 500. Map wstore.ErrNotFound to 404.

@@
-import (
+import (
 	"context"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"errors"
 	"log"
 	"net/http"
 	"strings"
@@
-	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
-	if err != nil {
-		http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
-		return
-	}
+	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
+	if err != nil {
+		if errors.Is(err, wstore.ErrNotFound) {
+			http.Error(w, "block not found", http.StatusNotFound)
+			return
+		}
+		http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+		return
+	}

Also applies to: 195-200

pkg/waveai/usechat-anthropic.go (3)

1-10: Clean up documentation artifacts and incorrect package preamble.

Remove :contentReference[...] tokens and fix package comment to match package waveai.

-// Package anthropicadapter streams Anthropic Messages API events and adapts them
+// Package waveai streams Anthropic Messages API events and adapts them
@@
-// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0}
+// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step)
@@
-// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2}
+// content_block_start/delta/stop, message_delta, message_stop, error).
@@
-// same shape. Adapt the import/alias as needed in your codebase. :contentReference[oaicite:3]{index=3}
+// same shape. Adapt the import/alias as needed in your codebase.
@@
-// Derived from anthropic-messages-api.md and anthropic-streaming.md. :contentReference[oaicite:6]{index=6} :contentReference[oaicite:7]{index=7}
+// Derived from anthropic-messages-api.md and anthropic-streaming.md.
@@
-// input_json_delta (concat, then parse once on content_block_stop). :contentReference[oaicite:8]{index=8}
+// input_json_delta (concat, then parse once on content_block_stop).
@@
-// - On Anthropic error event: AiMsgError and return StopKindError. :contentReference[oaicite:9]{index=9} :contentReference[oaicite:10]{index=10}
+// - On Anthropic error event: AiMsgError and return StopKindError.
@@
-endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages" // :contentReference[oaicite:11]{index=11}
+endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages"
@@
-// Request streaming SSE. :contentReference[oaicite:14]{index=14}
+// Request streaming SSE.
@@
-// Try to decode Anthropic error JSON schema then surface it. :contentReference[oaicite:15]{index=15}
+// Try to decode Anthropic error JSON schema then surface it.
@@
-// Event model: anthropic-streaming.md. :contentReference[oaicite:16]{index=16}
+// Event model: anthropic-streaming.md.
@@
-// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} :contentReference[oaicite:17]{index=17}
+// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}}
@@
-// ignore other block types gracefully per Anthropic guidance :contentReference[oaicite:18]{index=18}
+// ignore other block types gracefully per Anthropic guidance
@@
-// ignore; integrity metadata for thinking blocks. :contentReference[oaicite:19]{index=19}
+// ignore; integrity metadata for thinking blocks.
@@
-// ignore unknown deltas gracefully. :contentReference[oaicite:20]{index=20}
+// ignore unknown deltas gracefully.
@@
-// Unknown event names may appear over time; ignore. :contentReference[oaicite:22]{index=22}
+// Unknown event names may appear over time; ignore.
@@
-// body with stream=true and configured model/max_tokens. :contentReference[oaicite:23]{index=23}
+// body with stream=true and configured model/max_tokens.
@@
-// Content may be a string or array of blocks; support text only. :contentReference[oaicite:24]{index=24}
+// Content may be a string or array of blocks; support text only.
@@
-// Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25}
+// Shorthand: string becomes a single text block.

Also applies to: 69-70, 151-153, 189-196, 217-225, 260-261, 284-285, 388-389, 404-405, 468-469, 500-503, 596-599, 602-604, 618-619, 632-633


213-225: Validate and normalize BaseURL before building endpoint.

-	baseURL := opts.BaseURL
+	baseURL := opts.BaseURL
 	if baseURL == "" {
 		baseURL = "https://api.anthropic.com"
 	}
-	endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages"
+	u, uerr := url.Parse(baseURL)
+	if uerr != nil || u.Scheme == "" || u.Host == "" {
+		return nil, fmt.Errorf("invalid base URL: %q", baseURL)
+	}
+	endpoint := strings.TrimRight(u.String(), "/") + "/v1/messages"

550-556: Bug: message_stop reads transient stopFromDelta instead of persisted finalStop.

This can drop the stop_reason; use the function param finalStop.

-		reason := "end_turn"
-		if stopFromDelta != nil && *stopFromDelta != "" {
-			reason = *stopFromDelta
-		}
+		reason := "end_turn"
+		if finalStop != "" {
+			reason = finalStop
+		}
pkg/waveai/ssehandlerch.go (1)

365-370: Include finishReason and usage in AiMsgFinish payload.

 func (h *SSEHandlerCh) AiMsgFinish(finishReason string, usage interface{}) error {
 	resp := map[string]interface{}{
 		"type": AiMsgFinish,
 	}
+	if finishReason != "" {
+		resp["finishReason"] = finishReason
+	}
+	if usage != nil {
+		resp["usage"] = usage
+	}
 	return h.WriteJsonData(resp)
 }
🧹 Nitpick comments (5)
pkg/waveai/usechat.go (3)

172-177: CORS preflight likely incomplete; set allow headers/methods.

 if r.Method == http.MethodOptions {
-	w.WriteHeader(http.StatusOK)
+	w.Header().Set("Access-Control-Allow-Origin", "*")
+	w.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS")
+	w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
+	w.WriteHeader(http.StatusNoContent)
 	return
 }

If upstream middleware already sets CORS, ignore this; otherwise this fixes OPTIONS failing in browsers.


188-194: Limit request body size and close it.

Defend against large bodies and ensure r.Body is closed.

-// Parse request body completely before sending any response
-var req UseChatRequest
-if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+// Parse request body completely before sending any response
+r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MiB cap
+defer r.Body.Close()
+var req UseChatRequest
+if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
 	http.Error(w, fmt.Sprintf("Invalid request body: %v", err), http.StatusBadRequest)
 	return
 }

189-193: Validate non-empty messages early.

 if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
 	http.Error(w, fmt.Sprintf("Invalid request body: %v", err), http.StatusBadRequest)
 	return
 }
+if len(req.Messages) == 0 {
+	http.Error(w, "At least one message is required", http.StatusBadRequest)
+	return
+}
pkg/waveai/usechat-anthropic.go (2)

371-378: Emit finish with reason captured (not empty).

Sends finish with empty reason; prefer finalStop.

-	_ = sse.AiMsgFinish("", nil)
+	_ = sse.AiMsgFinish(finalStop, nil)

643-647: Handle crypto/rand errors in genID.

 func genID(prefix string) string {
 	var b [8]byte
-	_, _ = rand.Read(b[:])
+	if _, err := rand.Read(b[:]); err != nil {
+		return fmt.Sprintf("%s_%x", prefix, time.Now().UnixNano())
+	}
 	return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:]))
 }

And ensure time is imported if not already.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f944df9 and 09323e2.

📒 Files selected for processing (3)
  • pkg/waveai/ssehandlerch.go (1 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/waveai/usechat.go (7)
pkg/wconfig/settingsconfig.go (2)
  • AiSettingsType (35-52)
  • MergeAiSettings (165-229)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/wstore/wstore_dbops.go (1)
  • DBMustGet (138-149)
pkg/waveai/waveai.go (2)
  • APIType_OpenAI (21-21)
  • APIType_Anthropic (18-18)
pkg/waveai/ssehandlerch.go (2)
  • SSEHandlerCh (63-75)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-openai-completions.go (1)
  • StreamOpenAIChatCompletions (41-162)
pkg/waveai/usechat-openai-responses.go (1)
  • StreamOpenAIResponsesAPI (82-225)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/ssehandlerch.go (14)
  • SSEHandlerCh (63-75)
  • AiMsgError (52-52)
  • AiMsgFinish (51-51)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgReasoningStart (42-42)
  • AiMsgToolInputStart (45-45)
  • AiMsgTextDelta (40-40)
  • AiMsgReasoningDelta (43-43)
  • AiMsgToolInputDelta (46-46)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgToolInputAvailable (47-47)
  • AiMsgFinishStep (50-50)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (1)
  • UseChatMessage (27-31)
🔇 Additional comments (2)
pkg/waveai/usechat.go (1)

146-153: Do not remove the o1- branch — o1 models use Chat Completions (not Responses API)*

OpenAI's model-availability docs list the o1 family (o1, o1-preview, o1-pro, o1-mini) on the Chat Completions / Assistants / Batch endpoints, so keep the strings.HasPrefix(m, "o1-") branch in shouldUseChatCompletionsAPI.

Likely an incorrect or invalid review comment.

pkg/waveai/ssehandlerch.go (1)

102-108: LGTM: Cache-Control consolidated.

The duplicate Cache-Control header issue is resolved by a single combined directive.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
cmd/testai/main-testai.go (1)

100-139: Use the --tools flag in OpenAI path and pass tools to StreamOpenAIToUseChat

The withTools flag is ignored for OpenAI; tools are never constructed or passed, so tool flows aren’t exercised. Mirror the Anthropic path by creating tool definitions when withTools is true and pass them into the call.

 func testOpenAI(model, message string, withTools bool) {
@@
 	messages := []waveai.UseChatMessage{
 		{
 			Role:    "user",
 			Content: message,
 		},
 	}
 
+	var tools []waveai.ToolDefinition
+	if withTools {
+		tools = getToolDefinitions()
+		fmt.Printf("Tools enabled: %d tools configured\n", len(tools))
+	}
+
 	fmt.Printf("Testing OpenAI streaming with model: %s\n", model)
 	fmt.Printf("Message: %s\n", message)
 	fmt.Println("---")
@@
-	_, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, nil)
+	stopReason, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools)
 	if err != nil {
 		fmt.Printf("Error streaming OpenAI: %v\n", err)
 	}
+	if stopReason != nil {
+		fmt.Printf("Stop reason: %+v\n", stopReason)
+	}
pkg/waveai/usechat.go (2)

240-244: Tighten API type validation (don’t special-case empty string here)

You already default APIType to OpenAI in resolveAIConfig. Simplify validation to only accept known values.

-	// Support OpenAI and Anthropic
-	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
-		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+	// Support OpenAI and Anthropic
+	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
+		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
 		return
 	}

197-201: Handle crypto/rand.Read error in generateID (fallback or propagate)

rand.Read can fail; ignoring the error risks returning an all-zero ID. Add error handling and a safe fallback.

 func generateID() string {
 	bytes := make([]byte, 16)
-	rand.Read(bytes)
+	if _, err := rand.Read(bytes); err != nil {
+		// Fallback to time-based ID if CSPRNG fails
+		return fmt.Sprintf("%x", time.Now().UnixNano())
+	}
 	return hex.EncodeToString(bytes)
 }

Also add missing imports:

 import (
 	"context"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"errors"
 	"log"
 	"net/http"
 	"strings"
+	"time"
🧹 Nitpick comments (2)
pkg/waveai/usechat.go (2)

107-124: Avoid shadowing presetKey and use a clearer name for the preset map key

The inner var presetKey string shadows the function parameter presetKey, which is confusing and error-prone. Rename the inner variable to something distinct.

 	// Load preset configuration
 	var presetAiSettings *wconfig.AiSettingsType
 	if finalPreset != "default" {
-		var presetKey string
+		var presetMapKey string
 		if strings.HasPrefix(finalPreset, "ai@") {
-			presetKey = finalPreset
+			presetMapKey = finalPreset
 		} else {
-			presetKey = fmt.Sprintf("ai@%s", finalPreset)
+			presetMapKey = fmt.Sprintf("ai@%s", finalPreset)
 		}
-		if preset, ok := fullConfig.Presets[presetKey]; ok {
+		if preset, ok := fullConfig.Presets[presetMapKey]; ok {
 			presetAiSettings = &wconfig.AiSettingsType{}
 			if err := json.Unmarshal(mustMarshal(preset), presetAiSettings); err == nil {
 				// Successfully unmarshaled preset
 			} else {
 				presetAiSettings = nil
 			}
 		}
 	}

203-209: Preflight response lacks CORS headers

If CORS isn’t injected by an upstream middleware, OPTIONS should include Access-Control-Allow-* headers. Verify upstream handling; if absent, add them here.

Would you like me to add a minimal header set here guarded by a dev flag?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09323e2 and 4c54f6a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • cmd/testai/main-testai.go (1 hunks)
  • go.mod (2 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat-openai-responses.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • pkg/waveai/usechat-openai-responses.go
  • pkg/waveai/usechat-anthropic.go
  • go.mod
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
  • ToolDefinition (54-58)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (182-195)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (207-309)
pkg/waveai/usechat.go (8)
pkg/waveai/usechat-anthropic.go (3)
  • StopReasonKind (30-30)
  • StopKindDone (33-33)
  • StreamAnthropicResponses (207-309)
pkg/wconfig/settingsconfig.go (2)
  • AiSettingsType (35-52)
  • MergeAiSettings (165-229)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/wstore/wstore_dbops.go (1)
  • DBMustGet (138-149)
pkg/waveai/waveai.go (2)
  • APIType_OpenAI (21-21)
  • APIType_Anthropic (18-18)
pkg/waveai/ssehandlerch.go (2)
  • SSEHandlerCh (63-75)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-openai-completions.go (1)
  • StreamOpenAIChatCompletions (41-162)
pkg/waveai/usechat-openai-responses.go (1)
  • StreamOpenAIResponsesAPI (92-235)

Comment on lines +226 to +231
// Resolve AI configuration
aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
if err != nil {
http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return 404 when block is not found (preserve sentinel with errors.Is)

All configuration errors are mapped to 500. Distinguish wstore.ErrNotFound and return a proper 404.

-	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
-	if err != nil {
-		http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
-		return
-	}
+	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
+	if err != nil {
+		if errors.Is(err, wstore.ErrNotFound) {
+			http.Error(w, "Block not found", http.StatusNotFound)
+		} else {
+			http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+		}
+		return
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Resolve AI configuration
aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
if err != nil {
http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
return
}
// Resolve AI configuration
aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
if err != nil {
if errors.Is(err, wstore.ErrNotFound) {
http.Error(w, "Block not found", http.StatusNotFound)
} else {
http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
}
return
}
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 226 to 231, the handler currently maps all
resolveAIConfig errors to 500; change it to detect a not-found sentinel by using
errors.Is(err, wstore.ErrNotFound) and return http.StatusNotFound in that case,
otherwise keep returning http.StatusInternalServerError; ensure you preserve
sentinel semantics by using errors.Is (and add an import for the standard
"errors" package if missing).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (6)
cmd/testai/main-testai.go (1)

100-139: Hook up --tools for OpenAI path (parity with Anthropic).

withTools is ignored here, so OpenAI streaming never exercises tool support. Pass tools when requested and surface the stop reason (matches Anthropic test).

Apply:

 func testOpenAI(model, message string, withTools bool) {
@@
 	messages := []waveai.UseChatMessage{
 		{
 			Role:    "user",
 			Content: message,
 		},
 	}
 
+	var tools []waveai.ToolDefinition
+	if withTools {
+		tools = getToolDefinitions()
+		fmt.Printf("Tools enabled: %d tools configured\n", len(tools))
+	}
+
 	fmt.Printf("Testing OpenAI streaming with model: %s\n", model)
 	fmt.Printf("Message: %s\n", message)
 	fmt.Println("---")
@@
 	defer sseHandler.Close()
 
-	_, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, nil)
+	stop, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools)
 	if err != nil {
 		fmt.Printf("Error streaming OpenAI: %v\n", err)
 	}
+	if stop != nil {
+		fmt.Printf("Stop reason: %+v\n", stop)
+	}
 }
pkg/waveai/usechat.go (2)

197-201: Handle crypto/rand.Read errors in ID generation.

Avoid ignoring failures; provide a safe fallback.

Apply:

 func generateID() string {
 	bytes := make([]byte, 16)
-	rand.Read(bytes)
-	return hex.EncodeToString(bytes)
+	if _, err := rand.Read(bytes); err != nil {
+		// extremely unlikely; fall back to timestamp-based ID
+		return fmt.Sprintf("id_%x", time.Now().UnixNano())
+	}
+	return hex.EncodeToString(bytes)
 }

And add import:

-	"log"
+	"log"
+	"time"

240-244: Tighten API type validation (empty string shouldn’t slip through).

Defaulting to OpenAI earlier means this check can be simplified and made stricter.

Apply:

-	// Support OpenAI and Anthropic
-	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic && aiOpts.APIType != "" {
-		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only OpenAI and Anthropic supported)", aiOpts.APIType), http.StatusBadRequest)
+	// Support exactly 'openai' or 'anthropic'
+	if aiOpts.APIType != APIType_OpenAI && aiOpts.APIType != APIType_Anthropic {
+		http.Error(w, fmt.Sprintf("Unsupported API type: %s (only 'openai' and 'anthropic' supported)", aiOpts.APIType), http.StatusBadRequest)
 		return
 	}
pkg/waveai/usechat-anthropic.go (2)

1-10: Clean up package comment and doc artifacts.

Top comment names wrong package and several lines contain generated :contentReference markers. Remove artifacts and fix the package comment.

Apply:

-// Package anthropicadapter streams Anthropic Messages API events and adapts them
+// Package waveai streams Anthropic Messages API events and adapts them
 // to our AI‑SDK style SSE parts. Mapping is based on the AI‑SDK data stream
-// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step) :contentReference[oaicite:0]{index=0}
+// protocol (start/text-start/text-delta/text-end, reasoning-*, tool-input-*, finish, finish-step)
 // and Anthropic's Messages + Streaming event schemas (message_start,
-// content_block_start/delta/stop, message_delta, message_stop, error). :contentReference[oaicite:1]{index=1} :contentReference[oaicite:2]{index=2}
+// content_block_start/delta/stop, message_delta, message_stop, error).
@@
-// Derived from anthropic-messages-api.md and anthropic-streaming.md. :contentReference[oaicite:6]{index=6} :contentReference[oaicite:7]{index=7}
+// Derived from anthropic-messages-api.md and anthropic-streaming.md.
@@
-// input_json_delta (concat, then parse once on content_block_stop). :contentReference[oaicite:8]{index=8}
+// input_json_delta (concat, then parse once on content_block_stop).
@@
-// - On Anthropic error event: AiMsgError and return StopKindError. :contentReference[oaicite:9]{index=10}
+// - On Anthropic error event: AiMsgError and return StopKindError.
@@
-		// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}} :contentReference[oaicite:17]{index=17}
+		// Example: data: {"type":"error","error":{"type":"overloaded_error","message":"Overloaded"}}
@@
-			// ignore other block types gracefully per Anthropic guidance :contentReference[oaicite:18]{index=18}
+			// ignore other block types gracefully per Anthropic guidance
@@
-			// ignore; integrity metadata for thinking blocks. :contentReference[oaicite:19]{index=19}
+			// ignore; integrity metadata for thinking blocks.
@@
-		// Unknown event names may appear over time; ignore. :contentReference[oaicite:22]{index=22}
+		// Unknown event names may appear over time; ignore.
@@
-		// Content may be a string or array of blocks; support text only. :contentReference[oaicite:24]{index=24}
+		// Content may be a string or array of blocks; support text only.
@@
-			// Shorthand: string becomes a single text block. :contentReference[oaicite:25]{index=25}
+			// Shorthand: string becomes a single text block.

Also applies to: 47-49, 130-131, 166-176, 334-336, 398-400, 430-434, 528-529, 572-575, 588-593


545-551: Validate and normalize base URL.

Avoid concatenating unvalidated base URLs; parse and ensure scheme/host.

Apply:

-	baseURL := opts.BaseURL
-	if baseURL == "" {
-		baseURL = AnthropicDefaultBaseURL
-	}
-	endpoint := strings.TrimRight(baseURL, "/") + "/v1/messages"
+	baseURL := opts.BaseURL
+	if baseURL == "" {
+		baseURL = AnthropicDefaultBaseURL
+	}
+	u, err := url.Parse(baseURL)
+	if err != nil || u.Scheme == "" || u.Host == "" {
+		return nil, fmt.Errorf("invalid base URL: %q", baseURL)
+	}
+	endpoint := strings.TrimRight(u.String(), "/") + "/v1/messages"
pkg/waveai/usechat-openai-responses.go (1)

118-118: Gate debug prints behind dev mode.

Avoid unconditional stdout logging in production.

Apply:

-		fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+		if wavebase.IsDevMode() {
+			fmt.Printf("DEBUG: Received event type: %s\n", event.Type)
+		}
@@
-			fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+			if wavebase.IsDevMode() {
+				fmt.Printf("DEBUG: reasoning delta - reasoningEnded=%t, delta='%s'\n", reasoningEnded, reasoningDelta.Delta)
+			}
@@
-			fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+			if wavebase.IsDevMode() {
+				fmt.Printf("DEBUG: reasoning summary text done - reasoningStarted=%t, reasoningEnded=%t (not ending here, waiting for output_item.done)\n", reasoningStarted, reasoningEnded)
+			}
@@
-			// Log unhandled event types in dev mode
-			if wavebase.IsDevMode() {
+			// Log unhandled event types in dev mode
+			if wavebase.IsDevMode() {
 				fmt.Printf("DEBUG: Unhandled event type: %s\n", event.Type)
 			}

Also applies to: 134-134, 140-140, 207-211

🧹 Nitpick comments (6)
pkg/waveai/usechat.go (2)

77-81: Allow tool definitions in the HTTP request (optional).

If you intend to support ToolUse via HTTP, add Tools to the request DTO and pass through to backends.

Apply:

 type UseChatRequest struct {
 	Messages []UseChatMessage        `json:"messages"`
 	Options  *wconfig.AiSettingsType `json:"options,omitempty"`
+	Tools    []ToolDefinition        `json:"tools,omitempty"`
 }

And:

-	if aiOpts.APIType == APIType_Anthropic {
-		_, err := StreamAnthropicResponses(r.Context(), sseHandler, aiOpts, req.Messages, nil)
+	if aiOpts.APIType == APIType_Anthropic {
+		_, err := StreamAnthropicResponses(r.Context(), sseHandler, aiOpts, req.Messages, req.Tools)
 		if err != nil {
 			log.Printf("Anthropic streaming error: %v", err)
 		}
 	} else {
 		// Default to OpenAI
-		_, err := StreamOpenAIToUseChat(r.Context(), sseHandler, aiOpts, req.Messages, nil)
+		_, err := StreamOpenAIToUseChat(r.Context(), sseHandler, aiOpts, req.Messages, req.Tools)
 		if err != nil {
 			log.Printf("OpenAI streaming error: %v", err)
 		}
 	}

Also applies to: 260-272


182-195: Consider returning the real StopReason from OpenAI paths.

Wrapper currently always returns Kind=done, which hides tool_use/max_tokens endings on Responses API. Unify return semantics with Anthropic for observability.

pkg/waveai/usechat-anthropic.go (1)

615-619: Handle crypto/rand failure when generating IDs.

Add a minimal fallback; time is already imported.

Apply:

 func genID(prefix string) string {
 	var b [8]byte
-	_, _ = rand.Read(b[:])
+	if _, err := rand.Read(b[:]); err != nil {
+		return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano())
+	}
 	return fmt.Sprintf("%s_%s", prefix, hex.EncodeToString(b[:]))
 }
pkg/waveai/usechat-openai-responses.go (3)

92-98: Honor TimeoutMs like the Anthropic path.

Wrap ctx with a timeout when provided for parity and to prevent hanging streams.

Apply:

 func StreamOpenAIResponsesAPI(sseHandler *SSEHandlerCh, ctx context.Context, opts *wshrpc.WaveAIOptsType, messages []UseChatMessage, tools []ToolDefinition) {
+	if opts.TimeoutMs > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, time.Duration(opts.TimeoutMs)*time.Millisecond)
+		defer cancel()
+	}
 	client, req := createOpenAIRequest(opts, messages, tools)

And add import:

 import (
 	"context"
 	"fmt"
+	"time"
 	"strings"

19-33: Validate non-empty message input or document behavior.

If all messages are filtered out as empty, the request has an empty Input list. Consider validating earlier and returning an explicit error to callers, or inject a minimal system/user placeholder.

Also applies to: 59-90


19-33: Proxy support parity with Anthropic (optional).

If opts.ProxyURL is set, configure the OpenAI client with a custom HTTP client using that proxy.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09323e2 and 4c54f6a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • cmd/testai/main-testai.go (1 hunks)
  • go.mod (2 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat-openai-responses.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod
🧰 Additional context used
🧬 Code graph analysis (4)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
  • ToolDefinition (54-58)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (182-195)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (207-309)
pkg/waveai/usechat.go (8)
pkg/waveai/usechat-anthropic.go (3)
  • StopReasonKind (30-30)
  • StopKindDone (33-33)
  • StreamAnthropicResponses (207-309)
pkg/wconfig/settingsconfig.go (2)
  • AiSettingsType (35-52)
  • MergeAiSettings (165-229)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/wstore/wstore_dbops.go (1)
  • DBMustGet (138-149)
pkg/waveai/waveai.go (2)
  • APIType_OpenAI (21-21)
  • APIType_Anthropic (18-18)
pkg/waveai/ssehandlerch.go (2)
  • SSEHandlerCh (63-75)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-openai-completions.go (1)
  • StreamOpenAIChatCompletions (41-162)
pkg/waveai/usechat-openai-responses.go (1)
  • StreamOpenAIResponsesAPI (92-235)
pkg/waveai/usechat-anthropic.go (3)
pkg/waveai/usechat.go (4)
  • ToolDefinition (54-58)
  • StopReason (39-51)
  • UseChatMessage (27-31)
  • ToolCall (33-37)
pkg/waveai/ssehandlerch.go (14)
  • SSEHandlerCh (63-75)
  • AiMsgError (52-52)
  • AiMsgFinish (51-51)
  • AiMsgStart (38-38)
  • AiMsgTextStart (39-39)
  • AiMsgReasoningStart (42-42)
  • AiMsgToolInputStart (45-45)
  • AiMsgTextDelta (40-40)
  • AiMsgReasoningDelta (43-43)
  • AiMsgToolInputDelta (46-46)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgToolInputAvailable (47-47)
  • AiMsgFinishStep (50-50)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat-openai-responses.go (5)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/usechat.go (2)
  • UseChatMessage (27-31)
  • ToolDefinition (54-58)
pkg/waveai/ssehandlerch.go (9)
  • SSEHandlerCh (63-75)
  • AiMsgStart (38-38)
  • AiMsgReasoningStart (42-42)
  • AiMsgReasoningDelta (43-43)
  • AiMsgTextStart (39-39)
  • AiMsgTextDelta (40-40)
  • AiMsgTextEnd (41-41)
  • AiMsgReasoningEnd (44-44)
  • AiMsgFinish (51-51)
pkg/waveai/usechat-openai-completions.go (1)
  • OpenAIUsageResponse (35-39)
pkg/wavebase/wavebase.go (1)
  • IsDevMode (102-104)
🔇 Additional comments (1)
pkg/waveai/usechat-openai-responses.go (1)

77-83: isReasoningModel present — no changes needed.
Defined at pkg/waveai/openaibackend.go:32; referenced from usechat-openai-responses.go and usechat-openai-completions.go.

Comment on lines +226 to +231
// Resolve AI configuration
aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
if err != nil {
http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return 404 when block is not found (preserve sentinel).

resolveAIConfig wraps with %w, so wstore.ErrNotFound survives. Map it to 404 instead of 500.

Apply:

-	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
+	aiOpts, err := resolveAIConfig(r.Context(), blockId, presetKey, req.Options)
 	if err != nil {
-		http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+		if errors.Is(err, wstore.ErrNotFound) {
+			http.Error(w, "block not found", http.StatusNotFound)
+		} else {
+			http.Error(w, fmt.Sprintf("Configuration error: %v", err), http.StatusInternalServerError)
+		}
 		return
 	}

And add the import:

 import (
 	"context"
 	"crypto/rand"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
 	"strings"
🤖 Prompt for AI Agents
In pkg/waveai/usechat.go around lines 226-231, the code currently always returns
HTTP 500 on resolveAIConfig errors; since resolveAIConfig wraps errors with %w
and preserves the sentinel wstore.ErrNotFound, change the error handling to
check errors.Is(err, wstore.ErrNotFound) and return http.StatusNotFound in that
case, otherwise return http.StatusInternalServerError as before; add the
"errors" import (and ensure wstore is imported/available) so you can use
errors.Is to preserve the sentinel comparison.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
cmd/testai/main-testai.go (1)

135-139: Resolved: OpenAI test now forwards tools to the streamer.

This addresses the earlier note about unused tool definitions in the OpenAI path.

🧹 Nitpick comments (4)
cmd/testai/main-testai.go (4)

120-123: Surface tool count and StopReason for OpenAI to match Anthropic logs.

Small UX boost when debugging streaming and tool usage.

Apply this diff:

@@
   fmt.Printf("Testing OpenAI streaming with model: %s\n", model)
   fmt.Printf("Message: %s\n", message)
   fmt.Println("---")
+  if len(tools) > 0 {
+    fmt.Printf("Tools enabled: %d tools configured\n", len(tools))
+  }
@@
-  _, err = waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools)
-  if err != nil {
-    fmt.Printf("Error streaming OpenAI: %v\n", err)
-  }
+  stopReason, err := waveai.StreamOpenAIToUseChat(ctx, sseHandler, opts, messages, tools)
+  if err != nil {
+    fmt.Printf("Error streaming OpenAI: %v\n", err)
+  } else if stopReason != nil {
+    fmt.Printf("Stop reason: %+v\n", stopReason)
+  }

Also applies to: 135-139


124-127: Avoid hanging runs: add a timeout to the OpenAI test context.

Prevents indefinite waits if a stream never finishes.

Apply this diff:

-  ctx := context.Background()
-  testWriter := &TestResponseWriter{}
-  sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)
+  ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+  defer cancel()
+  testWriter := &TestResponseWriter{}
+  sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)

161-164: Also log tool count for Anthropic for parity.

Keeps both paths consistent when troubleshooting tools.

Apply this diff:

   fmt.Printf("Testing Anthropic streaming with model: %s\n", model)
   fmt.Printf("Message: %s\n", message)
   fmt.Println("---")
+  if len(tools) > 0 {
+    fmt.Printf("Tools enabled: %d tools configured\n", len(tools))
+  }

165-168: Add a timeout to the Anthropic test context.

Same rationale as OpenAI path.

Apply this diff:

-  ctx := context.Background()
-  testWriter := &TestResponseWriter{}
-  sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)
+  ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+  defer cancel()
+  testWriter := &TestResponseWriter{}
+  sseHandler := waveai.MakeSSEHandlerCh(testWriter, ctx)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c54f6a and 143df8c.

📒 Files selected for processing (3)
  • cmd/testai/main-testai.go (1 hunks)
  • pkg/waveai/usechat-anthropic.go (1 hunks)
  • pkg/waveai/usechat.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/waveai/usechat-anthropic.go
  • pkg/waveai/usechat.go
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
  • ToolDefinition (54-58)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (182-195)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (207-309)
🔇 Additional comments (2)
cmd/testai/main-testai.go (2)

49-58: Good: implements ResponseController-required methods on the test writer.

Providing SetWriteDeadline/SetReadDeadline and Flusher avoids ErrNotSupported from http.NewResponseController during SSE. Solid for a CLI harness.


60-99: Schema-driven tools look sane.

Graceful fallback to a minimal object schema when "config" is absent keeps tests resilient. No issues spotted.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
cmd/testai/main-testai.go (5)

60-66: Log schema parse errors to stderr.

Keeps SSE output on stdout clean and separates diagnostics.

Apply:

-    fmt.Printf("Error parsing schema: %v\n", err)
+    fmt.Fprintf(os.Stderr, "Error parsing schema: %v\n", err)

100-112: Propagate ctx deadline into opts.TimeoutMs (optional).

Some backends respect opts.TimeoutMs. Deriving it from the passed ctx keeps behavior consistent across providers.

Apply in both test functions after constructing opts:

 opts := &wshrpc.WaveAIOptsType{
     APIToken:  apiKey,
     Model:     model,
     MaxTokens: 1000,
 }
+if dl, ok := ctx.Deadline(); ok {
+    opts.TimeoutMs = int(time.Until(dl).Milliseconds())
+}

120-123: Optional: warn when tools may be ignored by older OpenAI models.

The OpenAI adapter routes some models via Chat Completions, which don’t use tools; a small notice helps avoid confusion.

Apply:

 fmt.Printf("Testing OpenAI streaming with model: %s\n", model)
 fmt.Printf("Message: %s\n", message)
 fmt.Println("---")
+if len(tools) > 0 {
+    fmt.Println("Note: Tools are only used with OpenAI Responses API models; Chat Completions models may ignore them.")
+}

186-205: Add a configurable timeout flag (optional).

Hard-coded 60s can be restrictive. Let users override and plumb it through ctx.

Apply:

 func main() {
-    var anthropic, tools bool
+    var anthropic, tools bool
+    var timeout time.Duration
     flag.BoolVar(&anthropic, "anthropic", false, "Use Anthropic API instead of OpenAI")
     flag.BoolVar(&tools, "tools", false, "Enable GitHub Actions Monitor tools for testing")
+    flag.DurationVar(&timeout, "timeout", 60*time.Second, "Overall request timeout (e.g., 30s, 2m)")
     flag.Parse()
@@
-    ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+    ctx, cancel := context.WithTimeout(context.Background(), timeout)
     defer cancel()

Consider also reflecting the chosen timeout into opts.TimeoutMs as suggested earlier.


212-216: Optional: confirm tool count when enabled.

Useful for quick sanity checks in CLI output.

Apply:

 if tools {
     toolDefs = getToolDefinitions()
+    fmt.Printf("Tools enabled: %d\n", len(toolDefs))
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 143df8c and 517eb44.

📒 Files selected for processing (1)
  • cmd/testai/main-testai.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
  • ToolDefinition (54-58)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (182-195)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (207-309)
🔇 Additional comments (6)
cmd/testai/main-testai.go (6)

67-76: Sane fallback when config schema is absent.

Good defensive default to an empty object.


77-98: Tool definitions align with ToolDefinition schema.

Names/descriptions are clear; update_config correctly reuses the embedded config schema.


134-141: OpenAI path now forwards tools — resolves prior gap.

Passing tools into StreamOpenAIToUseChat addresses the previously noted omission. Nicely done.


143-184: Anthropic streaming path looks solid.

Consistent setup with OpenAI path, prints stop reason, and leverages the same SSE writer.


20-22: Embedded schema path verified — no change required

testschema.json is present at cmd/testai/testschema.json, so //go:embed testschema.json is correct.


23-58: TestResponseWriter is compatible with http.ResponseController — Go version confirmed (go 1.24.6).

No-op Flush and SetRead/SetWriteDeadline are acceptable for a CLI test harness; approve the changes.

@sawka sawka merged commit 6e35544 into main Sep 12, 2025
2 of 7 checks passed
@sawka sawka deleted the sawka/ai-sdk branch September 12, 2025 19:56
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
cmd/testai/main-testai.go (1)

101-142: Past concern resolved: OpenAI path now wires tools through.

You now pass tools into StreamOpenAIToUseChat; this addresses the previously raised gap.

🧹 Nitpick comments (6)
cmd/testai/main-testai.go (6)

36-39: Avoid allocation and ensure readable output in Write.

Use %s with []byte directly and add a trailing newline so events don’t run together.

-func (w *TestResponseWriter) Write(data []byte) (int, error) {
-	fmt.Printf("SSE: %s", string(data))
+func (w *TestResponseWriter) Write(data []byte) (int, error) {
+	// %s accepts []byte; adding newline keeps output legible.
+	fmt.Printf("SSE: %s\n", data)
 	return len(data), nil
}

108-113: Propagate CLI timeout to provider opts for consistent cancellation.

Set TimeoutMs from ctx.Deadline so provider HTTP clients can honor it (Anthropic already wraps ctx; this keeps parity and future-proofs OpenAI paths).

 opts := &wshrpc.WaveAIOptsType{
 	APIToken:  apiKey,
 	Model:     model,
 	MaxTokens: 1000,
 }
+if deadline, ok := ctx.Deadline(); ok {
+	opts.TimeoutMs = int(time.Until(deadline).Milliseconds())
+}
 opts := &wshrpc.WaveAIOptsType{
 	APIToken:  apiKey,
 	Model:     model,
 	MaxTokens: 1000,
 }
+if deadline, ok := ctx.Deadline(); ok {
+	opts.TimeoutMs = int(time.Until(deadline).Milliseconds())
+}

Also applies to: 151-156


121-124: Surface tool usage and warn when models may ignore tools.

Given pkg/waveai/usechat.go only passes tools on the Responses API path, older Chat Completions models will ignore them; make this explicit to users.

 fmt.Printf("Testing OpenAI streaming with model: %s\n", model)
 fmt.Printf("Message: %s\n", message)
 fmt.Println("---")
+if len(tools) > 0 {
+	fmt.Printf("Tools enabled: %d definitions\n", len(tools))
+	// Tools are used on the Responses API path; Chat Completions models may ignore them.
+	fmt.Println("Note: some OpenAI Chat Completions models may ignore tools; prefer Responses API models (gpt-4.1, gpt-4o, o4).")
+}

Also applies to: 135-141


164-167: Parity: log tool count for Anthropic too.

Tiny UX improvement for consistency.

 fmt.Printf("Testing Anthropic streaming with model: %s\n", model)
 fmt.Printf("Message: %s\n", message)
 fmt.Println("---")
+if len(tools) > 0 {
+	fmt.Printf("Tools enabled: %d definitions\n", len(tools))
+}

187-192: Make timeout configurable via flag; default to 60s.

Some streams exceed 60s. Add --timeout to control both ctx and provider opts (with the earlier change).

-	var anthropic, tools bool
+	var anthropic, tools bool
+	var timeoutSec int
 	flag.BoolVar(&anthropic, "anthropic", false, "Use Anthropic API instead of OpenAI")
 	flag.BoolVar(&tools, "tools", false, "Enable GitHub Actions Monitor tools for testing")
+	flag.IntVar(&timeoutSec, "timeout", 60, "Request timeout in seconds (0 = no timeout)")
 	flag.Parse()
-	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	var ctx context.Context
+	var cancel context.CancelFunc
+	if timeoutSec > 0 {
+		ctx, cancel = context.WithTimeout(context.Background(), time.Duration(timeoutSec)*time.Second)
+	} else {
+		ctx, cancel = context.WithCancel(context.Background())
+	}
 	defer cancel()

Also applies to: 218-220


101-142: Reduce duplication between provider test functions.

Shared setup/teardown and logging can be factored into a small helper to keep drift down.

Example shape:

  • helper setupSSE(ctx) -> (*waveai.SSEHandlerCh, func())
  • helper printPreamble(provider, model, message, toolsCount)

Also applies to: 144-185

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 517eb44 and 84b08b2.

📒 Files selected for processing (1)
  • cmd/testai/main-testai.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/testai/main-testai.go (4)
pkg/waveai/usechat.go (3)
  • ToolDefinition (54-58)
  • UseChatMessage (27-31)
  • StreamOpenAIToUseChat (182-195)
pkg/wshrpc/wshrpctypes.go (1)
  • WaveAIOptsType (492-503)
pkg/waveai/ssehandlerch.go (1)
  • MakeSSEHandlerCh (78-86)
pkg/waveai/usechat-anthropic.go (1)
  • StreamAnthropicResponses (207-309)
🔇 Additional comments (1)
cmd/testai/main-testai.go (1)

61-99: Tool schema parsing and fallbacks look solid.

Graceful fallback when config schema is missing and clear descriptions. No changes needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant